CATROID-1106: GlideToPositionBrick by rinaschedl · Pull Request #5239 · Catrobat/Catroid

rinaschedl · 2026-08-22T06:28:14Z

https://catrobat.atlassian.net/browse/CATROID-1106

A new Brick (Feature) is created.
Brick Behavior:
The Brick has 3 options:
-random position
- the sprite glides to a random position
-touch position
- the player has to touch a position where the sprite then goes
-other sprite position
- the sprite glides (and at the moment follows) a other sprite till the other sprite stops.
- this part is not finally discussed: may we change it so that the sprite goes to the startposition from the other sprite and don't follow at all.

Program changes:
create 3 Actions and 3 Actionstest, GlideToPositionBrick, brick_glide_to_position
changes in BrickValue, ActionFactory

Commitlist during coding:
TestGlideToRandomPositionDestination Test; flact:red
TestGlideToRandomPositionDestination flact:green; createGlidetoRandomPositionAction;
Refactoring and knew Test: GlideBehavior, bove Tests are green
show Brick in Bricklist with the correct attributs. Attributes don't work yet.
changed all glide_to_.... to glide_to_position_... so to better distinguish between.
last commit before PairProgramming Session with Thomas
glideToTouchPosition ready, brick test by hand and the tests are grenn
refactor the ActionTest: canceled all deprecated lines; correct misspellings;
add GlideToOtherSpritePosition
refactor GlideToRandomPositionAction.kt
Refactoring all files that changed in the ticket Catroid-1106

Your checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

@@ -0,0 +1,75 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<!--


github-advanced-security

detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

hannesweilharter · 2026-08-22T16:39:01Z

please fix all warnings first and have a look at the Unit Test Report, there are failing testcases. Not sure if they are related to these changes. Then i can do a review :)

A new Brick (Feature) is created. The Brick has 3 options: -random position - the sprite glides to a random position -touch position - the player has to touch a position where the sprite then goes -other sprite position - the sprite glides and at the moment follows a other sprite till the other sprite stops. - this part is not finally discussed: may we change it so that the sprite goes to the startposition from the other sprite, if it moves too TestGlideToRandomPositionDestination Test; flact:red TestGlideToRandomPositionDestination flact:green; createGlidetoRandomPositionAction; Refactoring and knew Test: GlideBehavior, bove Tests are green show Brick in Bricklist with the correct attributs. Attributes don't work yet. changed all glide_to_.... to glide_to_position_... so to better distinguish between. last commit before PairProgramming Session with Thomas glideToTouchPosition ready, brick test by hand and the tests are grenn refactor the ActionTest: canceled all deprecated lines; correct misspellings; add GlideToOtherSpritePosition refactor GlideToRandomPositionAction.kt Refactoring all files that changed in the ticket Catroid-1106 Refactoring all files that changed in the ticket Catroid-1106

…sts suddently failed and we do not have a koin base class

…e problem with the testpipeline

…ction for the GlideToPosition actions. Warnings of deprecated stays in the ActionFactory

rinaschedl · 2026-09-09T18:30:18Z

@hannesweilharter please start with CodeReview.
the issue on the Quality Gate is discussed with Thomas and I shall ingnore it for now. There will be a ticket for the Action Faktory because to change the warnings, you have also some changing to the Unit Tests necessary and the workload is relativly high, therefor we changed the ActionFactory back, so you see the warnings but the tests mostly pass now. The 2 failing Tests has nothing to too with the changes I made in the code, so far I can see, so I guess there one of the flanky ones? Lokal all Tests passed, exept some of them who Thomas confirmed are broken. So we should be good.
Thanks
Katharina

hannesweilharter

Basically the new feature is working well, but i have a few suggestions, ptal into the inline comments :)
There are a few other things I noticed, please have a look:

Small Bug: Move new brick into a Forever brick with "Touch position": This works only once. I guess the correct behavior should be that the sprite should always move to the touch position when the user touches the screen.
The two failing testcases in BricksXmlSerializerTest on the CI are actually related to the changes. The reason is: there is no alias registered for GlideToPositionBrick. To fix it just add this line in XStreamSerializer: xstream.alias("brick", GlideToPositionBrick.class);
Please make sure the copyright year is up to date in every file
Please fix all the code style warnings.

hannesweilharter · 2026-09-18T14:23:31Z

why deleting this file?

I can't remember that I removed a file. What does this file normaly do?

hannesweilharter · 2026-09-18T14:47:04Z

whats the purpose of this file?

This file make sure that every coder, reviewer etc is using the same Java.Version in his IDE settings, so to reduce codeproblems according to different javaversions.

hannesweilharter · 2026-09-18T15:38:12Z

+			case BrickValues.GLIDE_TO_RANDOM_POSITION:
+				GlideToRandomPositionAction randomAction =
+						action(GlideToRandomPositionAction.class);
+				randomAction.setScope(scope);
+				randomAction.setDuration(duration.interpretFloat(scope));
+				return randomAction;


I would suggest setting the scope and duration after the switch statement to remove code duplication.

There is no duplicated code, because in each case you call a different instance, there you set the scope etc.

hannesweilharter · 2026-09-18T15:42:37Z

 data class Scope(
    val project: Project?,
-    val sprite: Sprite,
+    var sprite: Sprite,


why changing val to var?

hannesweilharter · 2026-09-18T15:47:14Z

IMO it would make sense to introduce a common GlideToPositionAction parent class to reduce duplicated code between the three position-based glide actions. They all store a start position and current position etc.
There is also an already existing GlideToAction class which might contain some overlapping gliding logic. I haven't looked into the concrete behavior in detail, so please check whether the existing class can be reused somehow.

hannesweilharter · 2026-09-18T15:50:58Z

+        if (spinnerSelection == BrickValues.GLIDE_TO_RANDOM_POSITION) {
+            spinner.setSelection(0)
+        }
+        if (spinnerSelection == BrickValues.GLIDE_TO_TOUCH_POSITION) {
+            spinner.setSelection(1)
+        }
+        if (spinnerSelection == BrickValues.GLIDE_TO_OTHER_SPRITE_POSITION) {
+            spinner.setSelection(destinationSprite)
+        }


Suggested change

if (spinnerSelection == BrickValues.GLIDE_TO_RANDOM_POSITION) {

spinner.setSelection(0)

}

if (spinnerSelection == BrickValues.GLIDE_TO_TOUCH_POSITION) {

spinner.setSelection(1)

}

if (spinnerSelection == BrickValues.GLIDE_TO_OTHER_SPRITE_POSITION) {

spinner.setSelection(destinationSprite)

}

when (spinnerSelection) {

BrickValues.GLIDE_TO_RANDOM_POSITION ->

spinner.setSelection(0)

BrickValues.GLIDE_TO_TOUCH_POSITION ->

spinner.setSelection(1)

BrickValues.GLIDE_TO_OTHER_SPRITE_POSITION ->

spinner.setSelection(destinationSprite)

}

applied changes;

hannesweilharter · 2026-09-18T15:53:27Z

+        if (string == context.getString(R.string.brick_glide_to_touch_position)) {
+            spinnerSelection = BrickValues.GLIDE_TO_TOUCH_POSITION
+            destinationSprite = null
+        }
+
+        if (string == context.getString(R.string.brick_glide_to_random_position)) {
+            spinnerSelection = BrickValues.GLIDE_TO_RANDOM_POSITION
+            destinationSprite = null
+        }


Suggested change

if (string == context.getString(R.string.brick_glide_to_touch_position)) {

spinnerSelection = BrickValues.GLIDE_TO_TOUCH_POSITION

destinationSprite = null

}

if (string == context.getString(R.string.brick_glide_to_random_position)) {

spinnerSelection = BrickValues.GLIDE_TO_RANDOM_POSITION

destinationSprite = null

}

when (string) {

context.getString(R.string.brick_glide_to_touch_position) -> {

spinnerSelection = BrickValues.GLIDE_TO_TOUCH_POSITION

destinationSprite = null

}

context.getString(R.string.brick_glide_to_random_position) -> {

spinnerSelection = BrickValues.GLIDE_TO_RANDOM_POSITION

destinationSprite = null

}

}

applied changes

hannesweilharter · 2026-09-18T15:54:57Z

                BrickValues.Y_POSITION,
-                BrickValues.GLIDE_SECONDS
-            )
+                GLIDE_SECONDS)


Suggested change

GLIDE_SECONDS)

BrickValues.GLIDE_SECONDS

to stay consistent

sonarqubecloud · 2026-09-19T04:47:46Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-advanced-security Bot found potential problems Aug 22, 2026

View reviewed changes

Comment thread catroid/src/main/res/layout/brick_glide_to_position.xml

@@ -0,0 +1,75 @@

<?xml version="1.0" encoding="utf-8"?>

<!--

github-advanced-security Bot found potential problems Aug 22, 2026

View reviewed changes

hannesweilharter added the Active Member Tickets that are assigned to members that are still currently active label Aug 23, 2026

rinaschedl force-pushed the Catroid-1106 branch from 2ba8a8c to 23b5c77 Compare August 25, 2026 08:02

rinaschedl added 6 commits August 25, 2026 10:11

Push new gradle-daemon vor JavaVersions

d8b412f

Integrated manuel start and stop of Koin in the tests, because the te…

a14b750

…sts suddently failed and we do not have a koin base class

Merged gradle und run settings from develop in Catroid-1106 to fix th…

46e390e

…e problem with the testpipeline

Back roll ActionFactory to development state and only integrate new a…

e05b005

…ction for the GlideToPosition actions. Warnings of deprecated stays in the ActionFactory

Back roll ActionFactory to development state and only integrate new a…

953caee

…ction for the GlideToPosition actions. Warnings of deprecated stays in the ActionFactory

ActionFactory line 1589 deleted

a710758

hannesweilharter requested changes Sep 18, 2026

View reviewed changes

Catroid-1106: Some easy changes - readability

7e70e11

Conversation

rinaschedl commented Aug 22, 2026 • edited by hannesweilharter Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Your checklist for this pull request

Uh oh!

github-advanced-security AI left a comment

Choose a reason for hiding this comment

Uh oh!

hannesweilharter commented Aug 22, 2026

Uh oh!

rinaschedl commented Sep 9, 2026

Uh oh!

hannesweilharter left a comment

Choose a reason for hiding this comment

Uh oh!

hannesweilharter Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hannesweilharter Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Sep 19, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rinaschedl commented Aug 22, 2026 •

edited by hannesweilharter

Loading

hannesweilharter Sep 18, 2026 •

edited

Loading

hannesweilharter Sep 18, 2026 •

edited

Loading